Skip to content

fix: audio dac (enters a loop while playing trims) - #7463

Merged
pfeerick merged 1 commit into
2.12from
3djc/fix-h7-audiodac
Jul 5, 2026
Merged

fix: audio dac (enters a loop while playing trims)#7463
pfeerick merged 1 commit into
2.12from
3djc/fix-h7-audiodac

Conversation

@3djc

@3djc 3djc commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

In some circumstances, the audio can enter in 'loop' state, as show in video

Video_1781608013802_110.mp4

This fixes it.

While I have bot been able to reproduce on main, I would think main needs it too (I have done PR on 2.12 since I could reproduce issue and confirm fix)

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5d0a62e0-211d-4f1b-ac0b-1f27c4d6bf5e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 3djc/fix-h7-audiodac

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@richardclli

Copy link
Copy Markdown
Member

Is this the same problem of PR #7274 ?

@pfeerick

pfeerick commented Jun 16, 2026

Copy link
Copy Markdown
Member

Is this the same problem of PR #7274 ?

I believe so, yes - or at least the PA01 and v12 have shown exactly same symptoms for the past several months.

@richardclli

richardclli commented Jun 18, 2026

Copy link
Copy Markdown
Member

seems both fixes are necessary, and these 2 are for dma underrun in H7 radios, I am tracking similar problem in F4 radios, which is a regression bug from 2.10 -> 2.11

@richardclli

Copy link
Copy Markdown
Member

My AI coder gives this comments for both PRs:

H7 DAC Audio DMA — PR #7274 vs PR #7463 Comparison

Context

Both PRs target the H5/H7/H7RS circular double-buffer DMA path in radio/src/targets/common/arm/stm32/audio_dac_driver.cpp. The H7 architecture uses a single _dma_buffer[640] split into two 320-sample halves, with DMA running in continuous circular mode. HT (Half Transfer) and TC (Transfer Complete) interrupts refill each half.

Neither PR affects the F4 path (PL18, Taranis, etc.) — they are H7-only fixes.

PR #7274 — "chore(radio): audio related DMA cleanup" (SchrodiL)

Problem solved

DMA thrashing: the old code stops DMA immediately when a single buffer half runs empty, then the audio task restarts it on the next cycle. This causes rapid stop/start jitter and audio gaps during brief buffer starvation (e.g., a flash page read that takes 1-2ms).

Solution

Adds a starvation hysteresis counter _empty_dma_halves with threshold DMA_EMPTY_HALVES_STOP_THRESHOLD = 6:

  • Each half-transfer that finds no data increments the counter
  • Each half-transfer that finds data resets the counter to 0
  • DMA is only stopped after 6 consecutive empty halves (~30ms of silence at 32kHz)
  • This absorbs transient underruns without stopping DMA

Additional changes

  • dac_clear_dma_flags() helper to flush stale HT/TC flags before enabling DMA
  • EN-bit busy-wait in dac_close_dma_xfer() to ensure hardware has fully disabled before flag check
  • Flag clearing in dac_start_dma() before enabling interrupts

Tradeoff

During the hysteresis window, if the audio producer is genuinely starved for >30ms, the circular DMA replays stale data from the other half-buffer for up to 30ms before stopping. This means a flash erase (45-400ms) still eventually stops cleanly, but there's a brief stale-audio period.

PR #7463 — "fix: audio dac (enters a loop while playing trims)" (3djc)

Problem solved

When DMA is stopped mid-transfer and restarted, the hardware registers M0AR (memory address) and NDTR (remaining transfer count) hold partial values — they point partway through _dma_buffer. On restart, HT fires at the wrong offset relative to the actual buffer halves, causing half-tracking to desync. This manifests as trim sounds looping endlessly.

Solution

  • dac_start_dma() now explicitly resets M0AR back to _dma_buffer[0] and NDTR to DMA_BUFFER_LEN on every restart
  • audioConsumeCurrentBuffer() primes both halves before starting DMA (audio_update_dma_buffer(0) + audio_update_dma_buffer(1)), so the first full cycle runs valid data with correct half alignment
  • DMA flags are cleared after stream disable to prevent carry-over

Additional changes

  • dac_close_dma_xfer() clears pending HT/TC flags (resolves the // TODO: reset flags comment)

Side-by-side comparison

PR #7274 (SchrodiL) PR #7463 (3djc)
Nature Preventative improvement Bug fix
Fixes a confirmed bug? No Yes (trim sound loop)
Reproduced? Design improvement Confirmed with video
Core mechanism _empty_dma_halves counter + threshold = 6 M0AR/NDTR reset + double-half prime
Effect on underrun Absorbs brief lags (≤30ms) without stopping No change to underrun detection
Effect on restart None Ensures clean half-tracking after restart
Stale audio during underrun? Up to 30ms of stale replay No (stop/restart is immediate)
Helps flash contention? Yes — a 1ms flash read won't stop audio No
DMA flag fix dac_clear_dma_flags() helper + EN-bit wait Direct flag clear in close + start
Conflict with other PR Overlaps on dac_close_dma_xfer() and dac_start_dma() Same overlap

Behavioral trace

Scenario: brief buffer lag (1-3 empty halves, e.g. flash page read)

  Time:   0ms     10ms     20ms     30ms     40ms
          │        │         │        │        │
  BOTH:   ◄── data ──► ◄── empty (x2) ──► ◄── data ──►
          audio plays     silent halves      recovers

  current main:
    ◄── data ──► [xfer ends] stop DMA [restart] ◄── data ──►
                  └── click/gap ──┘

  PR #7463 only:
    ◄── data ──► [xfer ends] stop DMA [restart cleanly] ◄── data
                  └── click/gap ──┘
        (same stop/start as main, but no desync on restart)

  PR #7274 only:
    ◄── data ──► [empty x2, _empty=2 < 6, keep running]
                 └── plays stale data from other half ──┘
                  └──→ data resumes, DMA never stopped
Scenario: long starvation (flash erase, 200ms)

  Time:   0ms     30ms     200ms
          │        │         │
  current main:
    ◄── data [stop] ◄── silence ◄── restart, recover
               ╰── 200ms silence ──╯

  PR #7463 only:
    ◄── data [stop] ◄── silence ◄── restart cleanly, recover
               ╰── 200ms silence ──╯
    (same as main but safe restart)

  PR #7274 only:
    ◄── data [empty x2] ◄── stale replay (30ms) [stop] ◄── silence [recover]
                    ╰── 30ms stale ╰── 170ms silence ──╯

Verdict

PR #7274 PR #7463
Priority Nice to have Must have
Apply order Second (after #7463) First
Criticality Quality improvement Fixes confirmed bug
Merge advice Apply both, resolve flag-clear overlap Apply both, resolve flag-clear overlap
Effect on PL18/F4 None (H7 only) None (H7 only)

Conclusion: They are complementary, not competing. PR #7463 ensures DMA never desyncs on restart. PR #7274 prevents unnecessary restarts in the first place through hysteresis. Both should be applied — #7463 first for the bug fix, then #7274 merged on top with the dac_close_dma_xfer() / dac_start_dma() overlap resolved.

The merged result would give:

  1. No thrashing on brief underruns (PR chore(radio): audio related DMA cleanup #7274)
  2. Clean restart with correct half-tracking after any stop (PR fix: audio dac (enters a loop while playing trims) #7463)
  3. Double-primed buffer halves for glitch-free start (PR fix: audio dac (enters a loop while playing trims) #7463)
  4. Robust DMA flag management on both start and stop (both)

@pfeerick

pfeerick commented Jun 22, 2026

Copy link
Copy Markdown
Member

Thanks for the breakdown / comparison. Obviously the AI review is incomplete / incorrect in thinking #7274 is cleanup only / not fixing the trim audio bug (#7227) , as that wasn't mentioned in the PR description itself, but they clearly target different aspects of the same issue.

@pfeerick pfeerick added this to the 2.12.3 milestone Jul 5, 2026
@pfeerick
pfeerick force-pushed the 3djc/fix-h7-audiodac branch from 26b08d2 to 5e0b90f Compare July 5, 2026 05:59
@pfeerick

pfeerick commented Jul 5, 2026

Copy link
Copy Markdown
Member

@3djc Just as a followup to this, can you see what is happening on the t22 (not sure if it affects other radios, ok pa01 also, so not just t22) around the trim sounds with "mute if no sound" enabled... (i.e. muted) ... on the 2.12 branch (and v2.12.2 release) it seems like it wants to mute for too short a time... when you do single clicks you don't hear the beep (or go past the centre to get the voice announcement)... just a clicks from the amplifier unmuting and re-muting, but if you hold the trim it will unmute for long enough to hear the beep. Beeps are fine though if you have "mute with no sound" disabled (so always unmuted).

@3djc

3djc commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

By the sound of it (lol), mute or unmute time must be wrong for that radio, will have a look

@pfeerick
pfeerick merged commit 0eeb136 into 2.12 Jul 5, 2026
52 checks passed
@pfeerick
pfeerick deleted the 3djc/fix-h7-audiodac branch July 5, 2026 06:15
@pfeerick pfeerick added bug 🪲 Something isn't working sound 🔉 Related to generated or played audio/sound labels Jul 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug 🪲 Something isn't working sound 🔉 Related to generated or played audio/sound

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants